-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SQLite implementation of query support for DB based index #1121
SQLite implementation of query support for DB based index #1121
Conversation
Test Results558 tests +8 554 ✔️ +8 7m 56s ⏱️ -10s Results for commit 5feaf04. ± Comparison against base commit 2f163c1. This pull request removes 10 and adds 18 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate all the comments, looks well thought out.
@@ -41,9 +63,31 @@ interface GetEntryOptions { | |||
useWorkInProgressIndex?: boolean; | |||
} | |||
|
|||
interface QueryResultsMeta { | |||
// TODO SQLite doesn't let us use cursors in the classic sense so we need to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by cursors in the classic sense? I am familiar with implementing cursor based pagination where the last value of the column from the previous query is selected as the cursor for the next set. This sounds like it would be achievable with SQLite also. Just wondering what kind of pagination you had in mind here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SQLite does have a cursor available, but it’s only available from the synchronous API.
Normally you get it like this:
import sqlite3;
con = sqlite3.connect("tutorial.db");
con.cursor()cur = con.cursor();
result = cur.execute("SELECT name FROM sqlite_master");
result.fetchone()
however from the async interface you no longer have a direct handle on the connection. Rather the connection is only available in the worker thread since it’s not possible for the connection to be serialized over the postMessage boundary.
The worker thread only has a couple API calls:
close(), config-get(), execute(), open(). https://sqlite.org/wasm/doc/trunk/api-worker1.md
the idea being only POJO’s are available to cross the postMessage boundary between a worker thread and the main thread
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So instead we’ll have to manually control the result set as I described in this comment. We’ll need to use the realm version so that we can prevent the index mutations made between pages from effecting the results (so the realm version will be part of the query), as well as only fetching one page at a time using the ROW_NUMBER() and/or LIMIT sql commands by having the caller keep track of what page number of results they are requesting and including that in our Query
interface.
I made a pagination linear ticket for this work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! I see now what you mean by the cursor and the boundary.
…ll value matching
…rch-query-support-in-sqlite-adapter
This PR provides a SQLite implementation of queries for our DB based index. A handy test index setup helper is also included in this PR. This PR only focuses on the
type
filter andeq
filter. And specifically nocontainsMany
field support yet.